Skip to content

Conversation

@bitnahian
Copy link
Contributor

@bitnahian bitnahian commented Jan 15, 2026

Pre-Review Checklist

  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • No breaking changes in accordance with the version policy.
  • Linting and type checking pass per make format and make typecheck.
  • PR title is fit for the release changelog.

Pre-Merge Checklist

  • New tests for any fix or new behavior, maintaining 100% coverage.
  • Updated documentation for new features and behaviors, including docstrings for API docs.

@dsfaccini dsfaccini added new models Support for new model(s) bedrock embeddings feature New feature request, or PR implementing a feature (enhancement) labels Jan 15, 2026
@dsfaccini dsfaccini changed the title feat: add BedrockEmbeddingModel for Nova, Cohere and Titan endpoints feat: add BedrockEmbeddingModel for Nova, Cohere and Titan endpoints Jan 15, 2026
str(response_body),
)

return EmbeddingResult(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets simplify this method by returning only the embeddings + tokens + in some cases the response ID, and then build the EmbeddingResult where we call parse_response. Right now when we combine single-document runs, we deconstruct the individual EmbeddingResults and turn them into one combined one anyway, so let's skip the intermediate EmbeddingResult. That also means we no longer have to pass inputs, model_name etc into this method anymore.

@bitnahian

This comment was marked as outdated.

…truncation strategies and supported purposes
- Removed the mock_vcr_botocore_content_length fixture to simplify VCR handling.
- Added 'x-amzn-bedrock-' to ALLOWED_HEADER_PREFIXES for Bedrock embeddings.
- Updated content-length header in serialize function to match decompressed body size.
- Adjusted input_tokens in RequestUsage for various Bedrock embedding tests to reflect actual token counts.
- Changed model names to include versioning for consistency.
- Increased embedding dimensions in test assertions for Bedrock and Nova models.
@bitnahian bitnahian requested a review from DouweM January 18, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author revision bedrock embeddings feature New feature request, or PR implementing a feature (enhancement) new models Support for new model(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants